WWSTCERT-9857 Add Zooz ZSE50 to zwave-siren (for WWST Cert) #2681
Conversation
| version: 1 | ||
| - id: battery | ||
| version: 1 | ||
| - id: firmwareUpdate |
There was a problem hiding this comment.
We don't typically include this capability for Z-Wave devices, since we don't do Z-Wave OTA.
There was a problem hiding this comment.
This solution was actually suggested by the WWST team who was working directly with Zooz ([email protected], not sure who exactly). It allows us to populate the currentVersion value and have it show in the UI. ST does not really offer any other way to expose that information. Zooz has many different firmware versions for each devices so when troubleshooting with a customer they need to know what firmware they have. The driver passed the Full Integration Test on the developer certification portal.
| capabilities.chime, | ||
| capabilities.powerSource, | ||
| capabilities.audioVolume, | ||
| capabilities.firmwareUpdate, |
There was a problem hiding this comment.
this can be omitted, as we have no default handling for the firmwareUpdate capability for Z-Wave
| @@ -0,0 +1,409 @@ | |||
| -- Copyright 2022 SmartThings | |||
| --- @param self st.zwave.Driver | ||
| --- @param device st.zwave.Device | ||
| local function update_firmwareUpdate_capability(self, device, component, major, minor) | ||
| if device:supports_capability_by_id(capabilities.firmwareUpdate.ID, component.id) then | ||
| local fmtFirmwareVersion = string.format("%d.%02d", major, minor) | ||
| device:emit_component_event(component, capabilities.firmwareUpdate.currentVersion({ value = fmtFirmwareVersion })) | ||
| end | ||
| end | ||
|
|
||
| --- Update the built in capability firmwareUpdate's currentVersion attribute with the | ||
| --- Zwave version information received during pairing of the device. | ||
| --- @param self st.zwave.Driver | ||
| --- @param device st.zwave.Device | ||
| local function updateFirmwareVersion(self, device) | ||
| local fw_major = (((device.st_store or {}).zwave_version or {}).firmware or {}).major | ||
| local fw_minor = (((device.st_store or {}).zwave_version or {}).firmware or {}).minor | ||
| if fw_major and fw_minor then | ||
| update_firmwareUpdate_capability(self, device, device.profile.components.main, fw_major, fw_minor) | ||
| else | ||
| device.log.warn("Firmware major or minor version not available.") | ||
| end | ||
| end |
There was a problem hiding this comment.
unless you're using this somewhere, these can be omitted
There was a problem hiding this comment.
Both are being used, in the same file. Zooz wants to populate the firmware version where users can see it.
| --- @param device st.zwave.Device | ||
| local function device_init(driver, device) | ||
| device:send(Version:Get({})) | ||
| device:try_update_metadata({ profile = ZSE50_DEFAULT_PROFILE }) |
There was a problem hiding this comment.
the profile shouldn't be changing so you shouldn't need to reset it
| device:try_update_metadata({ profile = ZSE50_DEFAULT_PROFILE }) | ||
|
|
||
| if (device:get_field("TONES_DURATION") == nil or device:get_field("TONE_DEFAULT") == nil) then | ||
| rebuildTones(device) |
There was a problem hiding this comment.
How often are these values likely to change?
There was a problem hiding this comment.
They do get stored as persistent after being rebuilt. This particular check just makes sure it is not nil for any reason, especially when switching from one driver to another. I could possibly move the check to added instead if you think that would be better but I have pretty extensively tested it already how it is written.
Also, it could need to be rebuilt at any time, the user can plug the device into a PC and add or remove tones. There is an option in the modes menu to force a rebuild.
| tones_list[tone_id] = string.format("%s: %s (%ss)", tone_id, tone_name, duration) | ||
| tones_duration[tone_id] = duration | ||
| device:set_field("TONES_LIST_TMP", tones_list) | ||
| device:set_field("TONES_DURATION_TMP", tones_duration) |
There was a problem hiding this comment.
it seems a little redundant to use two maps here where the only unique information one of them includes is the tone_name. Could you just use one map like:
{ [tone_id] = {duration: X, name: Y} }
And construct the strings in the places you actually want to emit them?
Also, the value of capabilities.mode.supportedArguments will be identical to device:get_field("TONES_LIST") most of the time, so it seems especially redundant there.
greens
left a comment
greens
left a comment
There was a problem hiding this comment.
Please write and include some unit tests and remove your debug logging.
greens
changed the title
|
I will work on the remaining issues and follow up when complete. |
2 participants
Check all that apply
Type of Change
Checklist
Description of Change
Adding Zooz ZSE50 for WWST Certification
Summary of Completed Tests